Add Constant type/value properties #280

eugenesvk · 2023-10-06T10:47:33Z

This is some draft of the edits I've made (currently only to v26 as that's the version of the locally generated docs) to be able to extract a list of constants from a crate as mentioned in #279

Unresolved question:

currently I'm using serde json to de/ser the Type since there is no conversion to trustfall::FieldValue implemented for this type, but that seems clunky, so what's the easiest way to have a proper support of this conversion (is it only possible in the trustfall main crate?)
- it's clunky since on the receiving side I can only try_into_struct

  struct Output {
    // type_  	: rustdoc_types::Type, // Type→FieldValue not implemented, use ↓ json→string
    type_     	: String,
  }
let results:Vec<_> = trustfall::execute_query(&schema, adapter.clone(), query, variables.clone()).expect("failed to run query")
    .map(|row| row.try_into_struct::<Output>().expect("shape mismatch")).collect();

into a string field, can't make it deserialise from json automatically).

(haven't done any formatting/tests yet)

allow reading constant value/expr/is_literal and type (this one serialized to a json string since there is no impl to convert it to FieldValue)

obi1kenobi

Nicely done! Thanks for the PR, I'd be happy to merge something like this.

I have a few suggestions around how to get the type info out more conveniently, and around some of the docs structure. Nothing major, should be resolvable in just a couple of minutes.

Also, are you interested in associated constants as well, or just regular ones? If the associated constants are missing fields, I'd be happy to take a PR there as well.

src/rustdoc_schema.graphql

Co-authored-by: Predrag Gruevski <2348618+obi1kenobi@users.noreply.github.com>

to avoid repeating said example

eugenesvk · 2023-10-06T18:32:19Z

Also, are you interested in associated constants as well, or just regular ones? If the associated constants are missing fields, I'd be happy to take a PR there as wel

just the regular ones, the associated ones already have the default value though seems to miss the type_ field, so added that as well

AssocConst
Fields
type_: Type
default: Option<String>

(temporary in the json string format)

src/rustdoc_schema.graphql

until trustfall adds proper support for custom scalar types

obi1kenobi

Just a couple of nitpicks on docs and such, otherwise it's good to merge. I usually don't merge PRs marked "draft" so when you've had a chance to take one more look and feel it's good to merge on your end, move it out of the draft state and I'll give it another quick look as well and then merge + publish.

obi1kenobi · 2023-10-08T15:36:56Z

src/rustdoc_schema.graphql

+  If the constant is set:
+


minor formatting nit:

Suggested change

If the constant is set:

If the constant is set:

This is actually intentional since no space after the "header" breaks the formatting, so instead of 2 bullet points you get one line (at least in my Sublime editor with LSP plugin)

Not sure how it works in your wasm playground or any other tools

Let me know if you want to add an extra empty line before anyway (personally prefer a bit more compact, but up to you)

Ah nice, thanks for double-checking it in the editor.

Let's add the extra empty line before the block, though. I think that way it's a bit more readable when it's not rendered as markdown.

It's just the adding the newline, so I'll merge this as-is and add the newline in a separate PR and then cut a release so we can move faster.

src/rustdoc_schema.graphql

obi1kenobi · 2023-10-08T15:40:01Z

src/rustdoc_schema.graphql

-  trait BatchIterator<const MIN: usize> {
-    const SIZE: usize = 16;  // `"16"` is the default
-    const LOG_AS: &'static str = "batch";  // `"\"batch\""` is the default, including escaped quotes
-    const MIN_SIZE: usize = MIN;  // "MIN" is the default, referring to the other constant's name


In the original example, MIN_SIZE was referring to the const generic constant. In the new example it refers to a local sibling constant only, and there's no example with a const generic. Mind adding a const generic example as well?

restored MIN to be the const generic and the sibling is just a standalone NUM number

src/rustdoc_schema.graphql

Cargo.toml

src/adapter/properties.rs

Co-authored-by: Predrag Gruevski <2348618+obi1kenobi@users.noreply.github.com>

expr/value/is_literal

eugenesvk · 2023-10-08T21:29:28Z

Done the last thing I wanted - updated a test, though had to split logic for Constants with more properties like expr/value and for GlobalValues that don't have those and only test name/path as before

(although I've only run the local test for this updaed const crate, not the whole test suite)

Then pending the unresolved doc formatting edits should be more or less fine?

And as far as I understood, porting to other rustdocs versions is semi-auto-done with the help of your magic script?

obi1kenobi · 2023-10-09T03:35:21Z

Looks great. Going to merge this, add the newline in the docs, and cut a new release that you should be able to use.

Thanks for the help! Let me know how this all works out in your use case.

And as far as I understood, porting to other rustdocs versions is semi-auto-done with the help of your magic script?

Yup! I'll take care of porting it and cut a release for all the rustdoc versions that cargo-semver-checks currently supports: v24, v26, v27.

* add Constant property resolution function allow reading constant value/expr/is_literal and type (this one serialized to a json string since there is no impl to convert it to FieldValue) * add Constant property rustdocs example * add Constant property rustdoc schema * dep: add serde_json for Constant property Type * fix clippy * uglify with rustfmt * Update src/rustdoc_schema.graphql Co-authored-by: Predrag Gruevski <2348618+obi1kenobi@users.noreply.github.com> * squeeze doc comment * add a doc comment note referencing a more comprehensive example to avoid repeating said example * add type_ field to AssociatedConstant property resolution function (temporary in the json string format) * add type_ field to AssociatedConstant property rustdoc schema * split schema docs so that each Constant property is documented * update schema doc comment * update schema docs trait name * fix a typo in schema docs * remove type Assoc/Const property until trustfall adds proper support for custom scalar types * remove type Assoc/Const property from schema * fix a typo in schema docs Co-authored-by: Predrag Gruevski <2348618+obi1kenobi@users.noreply.github.com> * dep: remove serde_json since Constant property Type was removed Co-authored-by: Predrag Gruevski <2348618+obi1kenobi@users.noreply.github.com> * remove unstable rustdocs example for Constant properties Co-authored-by: Predrag Gruevski <2348618+obi1kenobi@users.noreply.github.com> * Update formatting of the schema docs Co-authored-by: Predrag Gruevski <2348618+obi1kenobi@users.noreply.github.com> * Update AssociatedConstant example to refer to a const generic * Update Constant test with extra properties expr/value/is_literal --------- Co-authored-by: Predrag Gruevski <2348618+obi1kenobi@users.noreply.github.com>

* add Constant property resolution function allow reading constant value/expr/is_literal and type (this one serialized to a json string since there is no impl to convert it to FieldValue) * add Constant property rustdocs example * add Constant property rustdoc schema * dep: add serde_json for Constant property Type * fix clippy * uglify with rustfmt * Update src/rustdoc_schema.graphql * squeeze doc comment * add a doc comment note referencing a more comprehensive example to avoid repeating said example * add type_ field to AssociatedConstant property resolution function (temporary in the json string format) * add type_ field to AssociatedConstant property rustdoc schema * split schema docs so that each Constant property is documented * update schema doc comment * update schema docs trait name * fix a typo in schema docs * remove type Assoc/Const property until trustfall adds proper support for custom scalar types * remove type Assoc/Const property from schema * fix a typo in schema docs * dep: remove serde_json since Constant property Type was removed * remove unstable rustdocs example for Constant properties * Update formatting of the schema docs * Update AssociatedConstant example to refer to a const generic * Update Constant test with extra properties expr/value/is_literal --------- Co-authored-by: Evgeny <eugenesvk@users.noreply.github.com>

obi1kenobi · 2023-10-09T04:48:11Z

New versions are available on crates.io!

eugenesvk added 4 commits October 6, 2023 17:17

add Constant property resolution function

fc75c49

allow reading constant value/expr/is_literal and type (this one serialized to a json string since there is no impl to convert it to FieldValue)

add Constant property rustdocs example

364c401

add Constant property rustdoc schema

2d08ae2

dep: add serde_json for Constant property Type

443c086

eugenesvk marked this pull request as draft October 6, 2023 10:47

eugenesvk added 2 commits October 6, 2023 17:52

fix clippy

6854eb9

uglify with rustfmt

5233132

eugenesvk mentioned this pull request Oct 6, 2023

New schema and lints for const and static values obi1kenobi/cargo-semver-checks#366

Closed

obi1kenobi reviewed Oct 6, 2023

View reviewed changes

src/rustdoc_schema.graphql Outdated Show resolved Hide resolved

src/rustdoc_schema.graphql Outdated Show resolved Hide resolved

src/rustdoc_schema.graphql Outdated Show resolved Hide resolved

obi1kenobi reviewed Oct 6, 2023

View reviewed changes

src/rustdoc_schema.graphql Show resolved Hide resolved

eugenesvk and others added 3 commits October 7, 2023 00:26

Update src/rustdoc_schema.graphql

024f7b7

Co-authored-by: Predrag Gruevski <2348618+obi1kenobi@users.noreply.github.com>

squeeze doc comment

2e665e5

add a doc comment note referencing a more comprehensive example

278ecea

to avoid repeating said example

eugenesvk added 4 commits October 7, 2023 01:53

add type_ field to AssociatedConstant property resolution function

7dcafb5

(temporary in the json string format)

add type_ field to AssociatedConstant property rustdoc schema

097a7f1

split schema docs so that each Constant property is documented

2cdd8c0

update schema doc comment

14be2e1

obi1kenobi reviewed Oct 6, 2023

View reviewed changes

src/rustdoc_schema.graphql Outdated Show resolved Hide resolved

src/rustdoc_schema.graphql Outdated Show resolved Hide resolved

src/rustdoc_schema.graphql Outdated Show resolved Hide resolved

eugenesvk added 4 commits October 7, 2023 16:20

update schema docs trait name

48c1d3d

fix a typo in schema docs

f52b2ae

remove type Assoc/Const property

9a48e86

until trustfall adds proper support for custom scalar types

remove type Assoc/Const property from schema

151ac1a

obi1kenobi reviewed Oct 8, 2023

View reviewed changes

eugenesvk and others added 5 commits October 9, 2023 03:25

fix a typo in schema docs

dc87d61

Co-authored-by: Predrag Gruevski <2348618+obi1kenobi@users.noreply.github.com>

dep: remove serde_json since Constant property Type was removed

0859104

Co-authored-by: Predrag Gruevski <2348618+obi1kenobi@users.noreply.github.com>

remove unstable rustdocs example for Constant properties

4950f7c

Co-authored-by: Predrag Gruevski <2348618+obi1kenobi@users.noreply.github.com>

Update formatting of the schema docs

c0748bc

Co-authored-by: Predrag Gruevski <2348618+obi1kenobi@users.noreply.github.com>

Update AssociatedConstant example to refer to a const generic

e0ced16

eugenesvk force-pushed the rustdoc-v26-const branch from ea49e69 to e0ced16 Compare October 8, 2023 20:52

eugenesvk marked this pull request as ready for review October 8, 2023 21:20

eugenesvk force-pushed the rustdoc-v26-const branch from c109df4 to e0a90fe Compare October 8, 2023 21:27

Update Constant test with extra properties

0f75831

expr/value/is_literal

eugenesvk force-pushed the rustdoc-v26-const branch from e0a90fe to 0f75831 Compare October 8, 2023 21:28

obi1kenobi enabled auto-merge (squash) October 9, 2023 03:35

Merge branch 'rustdoc-v26' into rustdoc-v26-const

e9209dc

obi1kenobi merged commit 85a4470 into obi1kenobi:rustdoc-v26 Oct 9, 2023
8 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Constant type/value properties #280

Add Constant type/value properties #280

eugenesvk commented Oct 6, 2023 •

edited

Loading

obi1kenobi left a comment

eugenesvk commented Oct 6, 2023 •

edited

Loading

obi1kenobi left a comment

obi1kenobi Oct 8, 2023

eugenesvk Oct 8, 2023

obi1kenobi Oct 9, 2023

obi1kenobi Oct 9, 2023

obi1kenobi Oct 8, 2023

eugenesvk Oct 8, 2023

eugenesvk commented Oct 8, 2023

obi1kenobi commented Oct 9, 2023

obi1kenobi commented Oct 9, 2023

Add Constant type/value properties #280

Add Constant type/value properties #280

Conversation

eugenesvk commented Oct 6, 2023 • edited Loading

obi1kenobi left a comment

Choose a reason for hiding this comment

eugenesvk commented Oct 6, 2023 • edited Loading

obi1kenobi left a comment

Choose a reason for hiding this comment

obi1kenobi Oct 8, 2023

Choose a reason for hiding this comment

eugenesvk Oct 8, 2023

Choose a reason for hiding this comment

obi1kenobi Oct 9, 2023

Choose a reason for hiding this comment

obi1kenobi Oct 9, 2023

Choose a reason for hiding this comment

obi1kenobi Oct 8, 2023

Choose a reason for hiding this comment

eugenesvk Oct 8, 2023

Choose a reason for hiding this comment

eugenesvk commented Oct 8, 2023

obi1kenobi commented Oct 9, 2023

obi1kenobi commented Oct 9, 2023

eugenesvk commented Oct 6, 2023 •

edited

Loading

eugenesvk commented Oct 6, 2023 •

edited

Loading